Add AdbRunner for adb CLI operations by rmarinho · Pull Request #283 · dotnet/android-tools

rmarinho · 2026-02-23T17:39:27Z

Summary

Wraps adb CLI operations for device management. Addresses #279.

Parsing/formatting/merging logic ported from dotnet/android GetAvailableAndroidDevices MSBuild task, enabling code sharing via the external/xamarin-android-tools submodule. See draft PR: dotnet/android#10880

Public API

public class AdbRunner
{
    // Constructor — requires full path to adb executable
    public AdbRunner (string adbPath, IDictionary<string, string>? environmentVariables = null);

    // Instance methods (async, invoke adb process)
    public Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (CancellationToken ct = default);
    public Task WaitForDeviceAsync (string? serial = null, TimeSpan? timeout = null, CancellationToken ct = default);
    public Task StopEmulatorAsync (string serial, CancellationToken ct = default);

    // Static helpers — public so dotnet/android can call without instantiating AdbRunner
    public static List<AdbDeviceInfo> ParseAdbDevicesOutput (IEnumerable<string> lines);
    public static AdbDeviceStatus MapAdbStateToStatus (string adbState);
    public static string BuildDeviceDescription (AdbDeviceInfo device, Action<TraceLevel, string>? logger = null);
    public static string FormatDisplayName (string avdName);
    public static List<AdbDeviceInfo> MergeDevicesAndEmulators (IReadOnlyList<AdbDeviceInfo> adbDevices, IReadOnlyList<string> availableEmulators, Action<TraceLevel, string>? logger = null);
}

Internal methods (not part of public API):

GetEmulatorAvdNameAsync — queries AVD name via adb emu avd name with TCP console fallback
ProcessUtils.ThrowIfFailed — shared exit code validation

Key Design Decisions

Constructor takes string adbPath: Callers pass the resolved path; no lazy Func<string?> indirection. Optional environmentVariables dictionary for ANDROID_HOME/JAVA_HOME/PATH.
Static parsing methods are public static so dotnet/android can call them without instantiating AdbRunner (e.g., GetAvailableAndroidDevices MSBuild task passes List<string> to ParseAdbDevicesOutput)
IEnumerable<string> overload: dotnet/android passes List<string> directly from output lines
Logger parameter: BuildDeviceDescription and MergeDevicesAndEmulators accept Action<TraceLevel, string>? — dotnet/android passes this.CreateTaskLogger() for MSBuild trace output
Regex with explicit state list: Uses \s+ separator to match one or more whitespace characters (spaces or tabs). Matches explicit known states with IgnoreCase. Daemon startup lines (*) are pre-filtered.
Exit code checking: ListDevicesAsync, WaitForDeviceAsync, and StopEmulatorAsync throw InvalidOperationException with stderr context on non-zero exit via ProcessUtils.ThrowIfFailed (internal)
MapAdbStateToStatus as switch expression: Simple value mapping uses C# switch expression for conciseness
Property patterns instead of null-forgiving: Uses is { Length: > 0 } patterns throughout for null checks on netstandard2.0 where string.IsNullOrEmpty() lacks [NotNullWhen(false)]
FormatDisplayName: Lowercases before ToTitleCase to normalize mixed-case input (e.g., "PiXeL" → "Pixel")
Environment variables via StartProcess: Runners pass env vars dictionary to ProcessUtils.StartProcess. AndroidEnvironmentHelper.GetEnvironmentVariables() builds the dict.

Tests

45 unit tests (AdbRunnerTests.cs):

ParseAdbDevicesOutput: real-world data, empty output, single/multiple devices, mixed states, daemon messages, IP:port, Windows newlines, recovery/sideload, tab-separated output
FormatDisplayName: underscores, title case, API capitalization, mixed case, special chars, empty
MapAdbStateToStatus: all known states + unknown (recovery, sideload)
MergeDevicesAndEmulators: no emulators, no running, mixed, case-insensitive dedup, sorting
Constructor: valid path, null/empty throws, sets properties
WaitForDeviceAsync: timeout validation (negative, zero)

4 integration tests (RunnerIntegrationTests.cs):

Run only when TF_BUILD=True or CI=true (case-insensitive), skipped locally
Require pre-installed JDK (JAVA_HOME) and Android SDK (ANDROID_HOME) on CI agent
Assert.Ignore when ANDROID_HOME missing (no bootstrap/network dependency)
Cover: constructor, ListDevicesAsync, WaitForDeviceAsync timeout, tool discovery

Review Feedback Addressed

Feedback	Commit	Details
Constructor: require `string adbPath`	`b68bea9`	Replace `Func<string?>` with `string adbPath`, remove lazy resolution
Port device listing from dotnet/android	`669121a`, `c142198`, `ca1b3a2`	ParseAdbDevicesOutput, BuildDeviceDescription, FormatDisplayName, MergeDevicesAndEmulators
`ThrowIfFailed` overload + delete string `ParseAdbDevicesOutput`	`1c79b8b`	StringWriter overload delegates to string version; single `IEnumerable<string>` parse method
Replace null-forgiving `!` with property patterns	`3cadb58`	`is { Length: > 0 }` patterns in AdbRunner + AndroidEnvironmentHelper; `MapAdbStateToStatus` → switch expression
Remove section separator comments	`e66b6d4`	Removed `// ── Section ──` region-style comments
Tighten `RequireCi()` to check truthy value	`e66b6d4`	`string.Equals("true", OrdinalIgnoreCase)` instead of presence check
Remove bootstrap fallback in tests	`e66b6d4`	`Assert.Ignore` when `ANDROID_HOME` missing — no network-dependent SDK download
Fix regex comment accuracy	`e66b6d4`	Comment matches `\s+` behavior (1+ whitespace, not "2+ spaces")
Log exceptions in bare catch	`7801c18`	`GetEmulatorAvdNameAsync` logs via `Trace.WriteLine`
Update `copilot-instructions.md`	`3cadb58`	Added rules: no null-forgiving `!`, prefer switch expressions

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

jonathanpeppers · 2026-02-23T18:08:36Z

I'd like to get the System.Diagnostics.Process code unified like mentioned here:

Add base Android tool runner infrastructure #281 (comment)

Addresses PR #283 feedback to use existing ProcessUtils instead of the removed AndroidToolRunner. Simplifies API: - Methods now throw InvalidOperationException on failure - Uses ProcessUtils.RunToolAsync() for all tool invocations - Added AndroidDeviceInfo model - Removed complex ToolRunnerResult wrapper types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Addresses PR #283/#284 feedback to use existing ProcessUtils. Simplifies API by throwing exceptions on failure instead of returning result types with error states. Changes: - AdbRunner: Simplified using ProcessUtils.RunToolAsync() - EmulatorRunner: Uses ProcessUtils.StartToolBackground() - Removed duplicate AndroidDeviceInfo from Models directory Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

rmarinho · 2026-03-03T16:45:21Z

Implemented your suggested approach:

Ported the device listing logic from dotnet/android's GetAvailableAndroidDevices MSBuild task into AdbRunner in feature/adb-runner branch
AdbDeviceInfo now has all the same fields: Serial, Description, Type (enum), Status (enum), AvdName, Model, Product, Device, TransportId
Ported ParseAdbDevicesOutput (same regex pattern), BuildDeviceDescription (same priority order), FormatDisplayName (title case + API capitalization), MapAdbStateToStatus, and MergeDevicesAndEmulators (dedup + sorting)
Added GetEmulatorAvdNameAsync (async version of GetEmulatorAvdName)
33 unit tests ported from the dotnet/android test cases (parsing, display name formatting, status mapping, merging/dedup, path discovery)

Next steps per your plan:

✅ feature/adb-runner has the ported logic (pushed)
⬜ Open a draft PR in dotnet/android that updates the submodule + rewrites GetAvailableAndroidDevices.cs to consume the new shared API
⬜ Review/merge android-tools first, then dotnet/android

rmarinho · 2026-03-03T17:02:39Z

The dotnet/android side is now ready as a draft PR: dotnet/android#10880

It delegates GetAvailableAndroidDevices parsing/formatting/merging to the shared AdbRunner methods from this PR, removing ~200 lines of duplicated code. All 33 existing tests are preserved and updated to use AdbRunner/AdbDeviceInfo directly (no more reflection).

Workflow:

Merge this PR first
Update the dotnet/android submodule pointer from feature/adb-runner to main
Take Use shared AdbRunner from android-tools for device listing android#10880 out of draft and merge

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

src/Xamarin.Android.Tools.AndroidSdk/Runners/AndroidEnvironmentHelper.cs

src/Xamarin.Android.Tools.AndroidSdk/Models/AdbDeviceInfo.cs

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

…eout - Broaden AdbDevicesRegex to match any device state (recovery, sideload, etc.) using \s{2,} separator to avoid matching random text lines - Skip daemon startup lines (starting with *) in ParseAdbDevicesOutput - ListDevicesAsync now captures stderr and throws on non-zero exit code - WaitForDeviceAsync now checks exit code and throws with stdout/stderr context - Validate timeout: reject negative and zero TimeSpan values - Add 6 tests: recovery/sideload parsing, state mapping, timeout validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jonathanpeppers

🤖 AI Review Summary

Found 1 issue: error handling.

Error handling: Bare catch block in GetEmulatorAvdNameAsync silently swallows exceptions without logging (AdbRunner.cs:119)

👍 Excellent PR overall — thorough test coverage (44 unit + 9 integration tests), consistent use of ProcessUtils for all process creation, proper CancellationToken propagation throughout, correct OperationCanceledException rethrow before the bare catch, good use of EnvironmentVariableNames constants, and clean code organization (one type per file, file-scoped namespaces). The refactoring of SdkManager.GetEnvironmentVariables into the shared AndroidEnvironmentHelper is a nice DRY improvement.

This review was generated by the android-tools-reviewer skill based on review guidelines established by @jonathanpeppers.

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

rmarinho · 2026-03-04T20:11:15Z

Addressed the bare catch feedback in 9368682: GetEmulatorAvdNameAsync now catches Exception ex and logs via Trace.WriteLine instead of silently swallowing. Also updated PR description to accurately reflect the current API surface (including IEnumerable<string> overload, logger parameters, internal methods).

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

You can also share your feedback on Copilot code review. Take the survey.

tests/Xamarin.Android.Tools.AndroidSdk-Tests/RunnerIntegrationTests.cs

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

This skill let's you say: review this PR: #284 Some example code reviews: * #283 (review) * #284 (review) This is built off a combination of previous code reviews, saved in `docs/CODE_REVIEW_POSTMORTEM.md`, and the review rules in `references/review-rules.md`.

This skill lets you say: review this PR: #284 Some example code reviews: * #283 (review) * #284 (review) This is built off a combination of previous code reviews, saved in `docs/CODE_REVIEW_POSTMORTEM.md`, and the review rules in `references/review-rules.md`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

jonathanpeppers · 2026-03-04T22:59:29Z

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

+			using var client = new TcpClient ();
+			using var cts = CancellationTokenSource.CreateLinkedTokenSource (cancellationToken);
+			cts.CancelAfter (TimeSpan.FromSeconds (3));
+
+			// ConnectAsync with CancellationToken not available on netstandard2.0;
+			// use Task.Run + token check instead
+			var connectTask = client.ConnectAsync ("127.0.0.1", port);
+			var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);
+			if (completed != connectTask) {
+				return null;
+			}
+			await connectTask.ConfigureAwait (false); // observe exceptions


What is going on here? Why are we using a TcpClient?!?

The TcpClient fallback exists because adb -s emulator-XXXX emu avd name returns exit code 0 but empty output on macOS. We verified this with both:

android-tools AdbRunner (empty AvdName without fallback)
dotnet/android GetAvailableAndroidDevices MSBuild task on main (same bug — Description=sdk gphone64 arm64 with no AVD name)
Without the AVD name, MergeDevicesAndEmulators can't match running emulators with AVD definitions from emulator -list-avds, leading to duplicates in UI device pickers.

The TCP console approach connects to the emulator's console port (emulator-{port}) and sends avd name — this works reliably because the emulator console protocol is stable and documented.

An alternative is process scanning (lsof -i :port → PID → ps -o command= → parse -avd arg), which avoids potential auth token requirements. Happy to switch to that approach if preferred, or keep TCP as the simpler cross-platform option. Let me know which you prefer.

The BootAndroidEmulator MSBuild task doesn't have this, so why do we need it here?

https://github.com/dotnet/android/blob/main/src/Xamarin.Android.Build.Tasks/Tasks/BootAndroidEmulator.cs

Again, I would avoid introducing TcpClient code if we don't need it. It doesn't seem like we should be opening network connections for this.

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

src/Xamarin.Android.Tools.AndroidSdk/Runners/AndroidEnvironmentHelper.cs

tests/Xamarin.Android.Tools.AndroidSdk-Tests/RunnerIntegrationTests.cs

Delegates adb devices parsing, description building, and device/emulator merging from GetAvailableAndroidDevices to AdbRunner in the shared xamarin-android-tools submodule. Removes ~200 lines of duplicated logic. - ParseAdbDevicesOutput accepts IEnumerable<string> to avoid string.Join - BuildDeviceDescription/MergeDevicesAndEmulators accept optional Action<TraceLevel, string> logger for MSBuild diagnostics - Tests updated to use AdbRunner/AdbDeviceInfo directly Depends on dotnet/android-tools#283. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rmarinho · 2026-03-05T12:18:47Z

Feedback addressed in `8e4be7a`

Threads 41-43: Simplify AdbRunner constructor

Replaced Func<string?> getSdkPath with string adbPath — the constructor now takes the full path to the adb executable. Removed:

AdbPath property
IsAvailable property
RequireAdb()
PATH discovery fallback (FindExecutablesInPath)
getSdkPath/getJdkPath fields

Environment variables can optionally be passed via the constructor parameter IDictionary<string, string>? environmentVariables.

Thread 44: TcpClient AVD name fallback — analysis

The TcpClient fallback exists because adb -s emulator-XXXX emu avd name returns exit code 0 but empty output on macOS. We verified this with both:

android-tools AdbRunner (empty AvdName without fallback)
dotnet/android GetAvailableAndroidDevices MSBuild task on main (same bug — Description=sdk gphone64 arm64 with no AVD name)

Without the AVD name, MergeDevicesAndEmulators can't match running emulators with AVD definitions from emulator -list-avds, leading to duplicates in UI device pickers.

The TCP console approach connects to the emulator's console port (emulator-{port}) and sends avd name — this works reliably because the emulator console protocol is stable and documented.

An alternative is process scanning (lsof -i :port → PID → ps -o command= → parse -avd arg), which avoids potential auth token requirements. Happy to switch to that approach if preferred, or keep TCP as the simpler cross-platform option. Let me know which you prefer.

Port adb device parsing, formatting, and merging logic from dotnet/android GetAvailableAndroidDevices MSBuild task into a shared AdbRunner class. New files: - AdbRunner.cs — ListDevicesAsync, WaitForDeviceAsync, StopEmulatorAsync, ParseAdbDevicesOutput, BuildDeviceDescription, FormatDisplayName, MapAdbStateToStatus, MergeDevicesAndEmulators - AdbDeviceInfo/AdbDeviceType/AdbDeviceStatus — device models - AndroidEnvironmentHelper — shared env var builder for all runners - ProcessUtils.ThrowIfFailed — shared exit code validation Modified files: - EnvironmentVariableNames — add ANDROID_USER_HOME, ANDROID_AVD_HOME - SdkManager.Process.cs — deduplicate env var logic via AndroidEnvironmentHelper Tests: - 43 unit tests (parsing, formatting, merging, path discovery, timeout) - 5 integration tests (CI-only, real SDK tools) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- StopEmulatorAsync now captures stderr and calls ThrowIfFailed for consistency with ListDevicesAsync/WaitForDeviceAsync. - ThrowIfFailed changed from public to internal since it is only used within the library. - Remove inaccurate cmdline-tools bootstrap claim from integration test doc comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@jonathanpeppers

Avoids allocating a joined string when callers already have individual lines (e.g., MSBuild LogEventsFromTextOutput). The existing string overload now delegates to the new one. Addresses review feedback from @jonathanpeppers on dotnet/android#10880. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…esAndEmulators Accepts Action<TraceLevel, string> to route debug messages through the caller's logging infrastructure (e.g., MSBuild TaskLoggingHelper). Restores log messages lost when logic moved from dotnet/android to android-tools: AVD name formatting, running emulator detection, and non-running emulator additions. Follows the existing CreateTaskLogger pattern used by JdkInstaller. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dotnet/android compiles the submodule as netstandard2.0 with WarningsAsErrors=Nullable. In netstandard2.0, string.IsNullOrEmpty lacks [NotNullWhen(false)], so the compiler doesn't narrow string? to string after null checks. Add null-forgiving operators where the preceding guard guarantees non-null. Fixes: CS8601 in AndroidEnvironmentHelper.cs (sdkPath, jdkPath) Fixes: CS8620 in AdbRunner.cs (serial in string[] array literal) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Replace \s{2,} with \s+ to handle tab-separated adb output - Use explicit state list (device|offline|unauthorized|etc.) instead of \S+ to prevent false positives from non-device lines - Add ParseAdbDevicesOutput_TabSeparator test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address review feedback: replace bare catch with catch(Exception ex) and log via Trace.WriteLine for debuggability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When 'adb emu avd name' fails (common on macOS), fall back to querying the emulator console directly via TCP on the console port extracted from the serial (emulator-XXXX -> port XXXX). This fixes duplicate device entries when running emulators can't be matched with their AVD definitions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address review feedback (threads 41-43): replace Func<string?> getSdkPath constructor with string adbPath that takes the full path to the adb executable. Remove AdbPath property, IsAvailable property, RequireAdb(), PATH discovery fallback, and getSdkPath/getJdkPath fields. Callers are now responsible for resolving the adb path before constructing. Environment variables can optionally be passed via the constructor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…seAdbDevicesOutput Thread 46: Add ProcessUtils.ThrowIfFailed(int, string, StringWriter?, StringWriter?) overload that delegates to the string version. Update AdbRunner callers to pass StringWriter directly instead of calling .ToString() at each call site. Thread 47: Remove ParseAdbDevicesOutput(string) overload. Callers now split the string themselves and pass IEnumerable<string> directly. This removes the dual-signature confusion and aligns with dotnet/android's usage pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…pression patterns that give the compiler proper non-null flow on netstandard2.0. - Convert MapAdbStateToStatus from switch statement to switch expression. - Update copilot-instructions.md with both guidelines for future PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- RequireCi(): check for truthy value (case-insensitive 'true') instead of just variable presence - Remove SDK bootstrap fallback; Assert.Ignore when ANDROID_HOME missing to avoid flaky network-dependent CI runs - Remove section separator comments (region-style anti-pattern) - Fix regex comment to match actual \s+ behavior (1+ whitespace) - Replace null-forgiving ex! with ex?.Message pattern - Remove unused usings and bootstrappedSdkPath field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

You can also share your feedback on Copilot code review. Take the survey.

src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs

Copilot · 2026-03-05T15:55:45Z

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

+			var connectTask = client.ConnectAsync ("127.0.0.1", port);
+			var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);
+			if (completed != connectTask) {


In QueryAvdNameViaConsoleAsync, if the Task.Delay(..., cts.Token) completes due to cancellation/timeout, the method returns null without honoring cancellationToken (external cancellation should propagate as OperationCanceledException). Also, when returning early, connectTask may still fault later without being observed. Consider explicitly handling cancellation vs timeout, and ensure any non-awaited connectTask has its exceptions observed (or is awaited/continued) before returning.

Suggested change

var connectTask = client.ConnectAsync ("127.0.0.1", port);

var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);

if (completed != connectTask) {

var connectTask = client.ConnectAsync ("127.0.0.1", port);

_ = connectTask.ContinueWith (

t => {

// Observe exceptions when the task is not awaited (e.g. timeout path)

var _ = t.Exception;

},

CancellationToken.None,

TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,

TaskScheduler.Default);

var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);

if (completed != connectTask) {

if (cancellationToken.IsCancellationRequested)

throw new OperationCanceledException (cancellationToken);

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 23, 2026 17:39

rmarinho mentioned this pull request Feb 23, 2026

Add EmulatorRunner for emulator CLI operations #284

Open

Copilot started reviewing on behalf of rmarinho February 23, 2026 17:40 View session

This comment was marked as resolved.

Sign in to view

rmarinho added the copilot `copilot-cli` or other AIs were used to author this label Feb 23, 2026

rmarinho force-pushed the feature/adb-runner branch from bb947a5 to b08dd3f Compare February 24, 2026 14:11

rmarinho force-pushed the feature/adb-runner branch 4 times, most recently from d378294 to ec0675f Compare March 2, 2026 11:42

jonathanpeppers reviewed Mar 2, 2026

View reviewed changes

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs Outdated Show resolved Hide resolved

rmarinho force-pushed the feature/adb-runner branch 3 times, most recently from 923285f to 1cf8fc6 Compare March 3, 2026 14:35

rmarinho mentioned this pull request Mar 3, 2026

Use shared AdbRunner from android-tools for device listing dotnet/android#10880

Draft

rmarinho requested a review from Copilot March 3, 2026 17:22

Copilot started reviewing on behalf of rmarinho March 3, 2026 17:23 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

rmarinho force-pushed the feature/adb-runner branch from 193203e to 5f9a212 Compare March 3, 2026 18:23

rmarinho force-pushed the feature/adb-runner branch from be56ade to 1d82e36 Compare March 3, 2026 19:18

rmarinho requested a review from Copilot March 4, 2026 09:24

Copilot started reviewing on behalf of rmarinho March 4, 2026 09:24 View session

This comment was marked as outdated.

Sign in to view

jonathanpeppers reviewed Mar 4, 2026

View reviewed changes

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs Outdated Show resolved Hide resolved

rmarinho requested a review from Copilot March 4, 2026 20:13

Copilot started reviewing on behalf of rmarinho March 4, 2026 20:13 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

tests/Xamarin.Android.Tools.AndroidSdk-Tests/RunnerIntegrationTests.cs Outdated Show resolved Hide resolved

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs Show resolved Hide resolved

jonathanpeppers mentioned this pull request Mar 4, 2026

[skills] create a skill for reviewing PRs #292

Merged

jonathanpeppers reviewed Mar 4, 2026

View reviewed changes

rmarinho force-pushed the feature/adb-runner branch from 3d2361d to 3cadb58 Compare March 5, 2026 12:53

rmarinho and others added 12 commits March 5, 2026 15:37

Log exception in GetEmulatorAvdNameAsync bare catch block

f68600d

Address review feedback: replace bare catch with catch(Exception ex) and log via Trace.WriteLine for debuggability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rmarinho force-pushed the feature/adb-runner branch from e66b6d4 to da3000c Compare March 5, 2026 15:38

rmarinho requested review from Copilot and jonathanpeppers March 5, 2026 15:49

Copilot started reviewing on behalf of rmarinho March 5, 2026 15:50 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Apply suggestions from code review

72291e2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

-			var connectTask = client.ConnectAsync ("127.0.0.1", port);
-			var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);
-			if (completed != connectTask) {
+			var connectTask = client.ConnectAsync ("127.0.0.1", port);
+			_ = connectTask.ContinueWith (
+				t => {
+					// Observe exceptions when the task is not awaited (e.g. timeout path)
+					var _ = t.Exception;
+				},
+				CancellationToken.None,
+				TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
+				TaskScheduler.Default);
+			var completed = await Task.WhenAny (connectTask, Task.Delay (3000, cts.Token)).ConfigureAwait (false);
+			if (completed != connectTask) {
+				if (cancellationToken.IsCancellationRequested)
+					throw new OperationCanceledException (cancellationToken);

Conversation

rmarinho commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Public API

Key Design Decisions

Tests

Review Feedback Addressed

Uh oh!

This comment was marked as resolved.

Uh oh!

jonathanpeppers commented Feb 23, 2026

Uh oh!

Uh oh!

rmarinho commented Mar 3, 2026

Uh oh!

rmarinho commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

🤖 AI Review Summary

Uh oh!

Uh oh!

rmarinho commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

rmarinho Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rmarinho commented Mar 5, 2026

Feedback addressed in 8e4be7a

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

rmarinho commented Feb 23, 2026 •

edited

Loading

Feedback addressed in `8e4be7a`